Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: clj-kondo defc linter #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rschmukler
Copy link

Introduce a linter for the defc macro. It has the following features:

  • Ensure that a defc has a render definition
  • Ensure that only valid hooks can be defined inside defc (ie. hook, render, bind, event)
  • Ensure that events are defined with valid arity
  • Provide information to clj-kondo so that symbols from bind can be linted (eg. unused vars)

Ultimately this could be extended with other things (eg. ensuring that definitions are provided for events specified in render functions, linting hiccup, ensuring render uses <<, etc.) but this felt like a good-enough start.

linter_example

@thheller
Copy link
Owner

I have never used clj-kondo, so I have no idea what this is or how it works.

It'll likely take some time before I get a chance to figure this out. I like the idea though.

@rschmukler
Copy link
Author

Sure! I've done a fair bit of work w/ clj-kondo hooks so feel free to ask (here or Clojurians slack) if you have any questions or want any specific functionality.

@zeitstein
Copy link
Contributor

zeitstein commented Jul 14, 2023

This is great and something I've been putting off doing for a long time :) Thank you!

Some observations:

  • << can be used in place of render, so that should be supported.
  • Technically, event can be defined with arity 1-3. I use this often, to avoid e.g. (event :e [env _ _]). (It seems it will compile even with (event :e [] ...), though I then get an error related to get_hook_value when I trigger the event.)
  • Probably not important, but there is an expected ordering to hooks, iirc. Something like binds need to be placed before render/<<, event hooks can go both before and after.

After making minor modifications for the first two points above, my project is linted correctly. Thanks again!

@rschmukler rschmukler force-pushed the rs/clj-kondo-hook branch 3 times, most recently from ee72ff4 to ea6272e Compare July 14, 2023 15:38
@rschmukler
Copy link
Author

@zeitstein Thanks for the feedback! I have updated the PR to handle << and allow for arity 1-3 on event. I also added the order detection on ensuring that there are no binds after render.

@rschmukler
Copy link
Author

rschmukler commented Dec 15, 2024

Hey @thheller I wonder if you might reconsider merging this for those of us that do use clj-kondo - I just rebased it in case you're interested.

@rschmukler rschmukler force-pushed the rs/clj-kondo-hook branch 3 times, most recently from 9ed594c to be660de Compare December 15, 2024 23:57
@rschmukler
Copy link
Author

I ended up updating this to also support effect hooks with the following:

  1. Checks that effect's "when" is either: :mount, :render, :auto or a vector of dependencies.
  2. Checks that the second argument to effect is an rarity 1 binding vector

@rschmukler
Copy link
Author

Also @thheller if you still aren't familiar with clj-kondo but want to see what this does... In a project with theller/shadow-grove pointed at my branch, you can run:

clj-kondo --lint $(clojure -Spath) --dependencies --skip-lint --copy-configs

Followed by

clj-kondo --lint src/

And you will get error messages for invalid usage of shadow-grove

@thheller
Copy link
Owner

I still have not looked into how clj-kondo works, or rather how it discovers these files. Do they actually need to be in the library .jar itself or just on the classpath?

I don't mind merging this, but I'd rather know what I'd get myself into beforehand. Also if anything skip the resources directory and move it into src/main instead.

Introduce a linter for the `defc` macro. It has the following features:

- Ensure that a `defc` has a `render` definition
- Ensure that only valid hooks can be defined inside defc (ie. `hook`,
  `render`, `bind`, `event`)
- Ensure that events are defined with valid arity
- Provide information to clj-kondo so that symbols from `bind` can be
  linted (eg. unused vars)

Ultimately this could be extended with other things (eg. ensuring that
definitions are provided for events specified in render functions) but
this felt like a good-enough start.
@rschmukler
Copy link
Author

@thheller clj-kondo works by looking for clj-kondo.exports/<org>/<project> on the class path of the library. Here's the section in the docs if you want to read more. Otherwise, I've moved it into main as requested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants